Skip to content

HIVE-29781: Iceberg: Basic stats fetch optimization - bulk partition stats read with query-scoped cache - #6662

Open
deniskuzZ wants to merge 3 commits into
apache:masterfrom
deniskuzZ:HIVE-29781
Open

HIVE-29781: Iceberg: Basic stats fetch optimization - bulk partition stats read with query-scoped cache#6662
deniskuzZ wants to merge 3 commits into
apache:masterfrom
deniskuzZ:HIVE-29781

Conversation

@deniskuzZ

@deniskuzZ deniskuzZ commented Jul 29, 2026

Copy link
Copy Markdown
Member

What changes were proposed in this pull request?

One partition-statistics read per query instead of one per partition

Why are the changes needed?

Compile-time regression on partitioned Iceberg tables

Does this PR introduce any user-facing change?

No

How was this patch tested?

TestHiveIcebergStatistics.java

@deniskuzZ deniskuzZ changed the title [WIP] HIVE-29781: Iceberg: Basic stats fetch optimization - bulk partition … HIVE-29781: Iceberg: Basic stats fetch optimization - bulk partition … Jul 31, 2026
@deniskuzZ
deniskuzZ marked this pull request as ready for review July 31, 2026 09:38
@deniskuzZ deniskuzZ changed the title HIVE-29781: Iceberg: Basic stats fetch optimization - bulk partition … HIVE-29781: Iceberg: Basic stats fetch optimization - bulk partition stats read Jul 31, 2026
@deniskuzZ deniskuzZ changed the title HIVE-29781: Iceberg: Basic stats fetch optimization - bulk partition stats read HIVE-29781: Iceberg: Basic stats fetch optimization - bulk partition stats read with query-scoped cache Jul 31, 2026
@deniskuzZ
deniskuzZ requested a review from Copilot July 31, 2026 12:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR optimizes Iceberg basic statistics fetching by introducing bulk, query-scoped partition stats reads (with caching) so that a single partition-stats read can serve the whole query instead of one read per partition. It also aligns ANALYZE/statistics flows and COUNT(*)-from-stats optimization with the new storage-handler backed stats model (including evolved/unpartitioned partitions).

Changes:

  • Add bulk basic-stats retrieval paths (HiveStorageHandler#getAggrBasicStatsFor, getRowCount(...)) and use them in planner/optimizer stats collection.
  • Rework BasicStats to accept explicitly provided stats without mutating HMS partition parameters; add tests for precedence.
  • Add query validation to reject partition-scoped ANALYZE for tables with non-native partition support; update Iceberg stats reading, caching, and golden outputs accordingly.

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
ql/src/test/org/apache/hadoop/hive/ql/stats/TestBasicStats.java Adds coverage for provided basic-stats precedence and immutability of partition parameters.
ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java Switches basic stats collection to use bulk storage-handler stats and table-level fallback.
ql/src/java/org/apache/hadoop/hive/ql/stats/BasicStatsTask.java Adjusts ANALYZE basic-stats collection to use table-scoped handler computation and non-native partition support behavior.
ql/src/java/org/apache/hadoop/hive/ql/stats/BasicStatsNoJobTask.java Makes NOSCAN handler-based stats computation table-scoped and safer for null returns.
ql/src/java/org/apache/hadoop/hive/ql/stats/BasicStats.java Introduces provided-stats construction path and avoids storage-handler-driven parameter mutation.
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java Rejects partition-clause ANALYZE where non-native partition support makes it unsafe.
ql/src/java/org/apache/hadoop/hive/ql/parse/PrunedPartitionList.java Clarifies semantics of “referred partition columns” used by pruning logic.
ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java Rejects partition-scoped column-stats ANALYZE for non-native partition support tables.
ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java Adds shared validation helper for unsupported partition clauses.
ql/src/java/org/apache/hadoop/hive/ql/optimizer/StatsOptimizer.java Updates COUNT(*)-from-stats path to use storage-handler row count APIs for non-native tables.
ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveStorageHandler.java Extends/adjusts storage handler API for table/partition row counts and batched basic stats retrieval.
ql/src/java/org/apache/hadoop/hive/ql/metadata/DummyPartition.java Adds a canonical synthetic “unpartitioned” partition name constant for legacy rows.
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/info/desc/DescTableOperation.java Updates DESCRIBE to use the new table-scoped storage handler basic stats API.
iceberg/iceberg-handler/src/test/results/positive/truncate_partitioned_iceberg_table.q.out Updates expected DESCRIBE output based on new stats population.
iceberg/iceberg-handler/src/test/results/positive/row_count.q.out Updates expected EXPLAIN output reflecting revised row-count derivation.
iceberg/iceberg-handler/src/test/results/positive/iceberg_truncate_partition_with_evolution.q.out Updates expected plans/outputs for evolved partitioning and revised stats behavior.
iceberg/iceberg-handler/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergStatistics.java Adds end-to-end tests for bulk stats, ANALYZE rejection, deletes, and evolved/unpartitioned stats behavior.
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/IcebergTableUtil.java Refactors partition stats reading to a single-pass file read keyed by consistent partition names.
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java Implements bulk partition basic stats, query-scoped cache, and exact row-count APIs for rewrite decisions.
common/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java Adds a dedicated error message for rejecting partition-clause ANALYZE in the new validation path.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java Outdated
Comment on lines +965 to +967
if (tbl.isPartitioned()) {
PrunedPartitionList prunedList = pctx.getPrunedPartitions(tsOp.getConf().getAlias(), tsOp);
if (!prunedList.getReferredPartCols().isEmpty()) {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The zero was deliberately moved inside the referredPartCols check. Hoisting it reintroduces a wrong answer on Iceberg tables converted from unpartitioned to partitioned: partition enumeration skips the former unpartitioned spec, so getPartitions() is empty while the table still has rows.
With this change applied — SELECT count(*) returns 0 while SELECT * returns 3 rows.

Comment thread common/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated no new comments.

Suppressed comments (2)

ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java:252

  • When the storage handler returns aggregated basic stats for only a subset of partitions, using Map.of() as the default forces BasicStats to ignore any existing HMS partition parameters for the missing partitions (because an empty providedStats is treated as authoritative). Passing null for missing entries lets BasicStats fall back to partish.getPartParameters() for those partitions while still using the batched stats for partitions that were returned.
        return inputs.stream()
            .map(pi -> factory.build(pi,
                aggrBasicStats.getOrDefault(pi.getPartition().getName(), Map.of())))
            .toList();

iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:924

  • metastoreRowCount can throw NumberFormatException when basic stats are marked up-to-date but the ROW_COUNT parameter is missing (e.g., computeBasicStatistics(..., quickStats=true) intentionally omits row count but callers still mark basic stats accurate). This method should return null when the row count is absent/unparseable, not throw.
  private static Long metastoreRowCount(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
    Map<String, String> parameters = hmsTable.getParameters();
    return StatsSetupConst.areBasicStatsUptoDate(parameters) ?
        NumberUtils.createLong(parameters.get(StatsSetupConst.ROW_COUNT)) : null;
  }

@sonarqubecloud

sonarqubecloud Bot commented Aug 1, 2026

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants